Update penumbra deps to v80 #48
Conversation
Required to resolve a build error when using Rust >1.80: error[E0282]: type annotations needed for `Box<_>` --> /home/conor/.cargo/registry/src/index.crates.io-6f17d22bba15001f/time-0.3.34/src/format_description/parse/mod.rs:83:9 | 83 | let items = format_items | ^^^^^ ... 86 | Ok(items.into()) | ---- type must be known at this point | = note: this is an inference error on crate `time` caused by an API change in Rust 1.80.0; update `time` to version `>=0.3.35` by calling `cargo update`
Updates the penumbra deps to commit [0] on main branch, including merge of [1]. The naive dep update breaks builds, so further changes are required. [0] ac7abacc9bb09503d6fd6a396bc0b6850079084e [1] penumbra-zone/penumbra#4905
Bumping cargo deps to resolve build errors.
The ViewServer init fn now requires an arg for registry path. Given type constraints, setting it to `None` requires pulling in `camino` as a dep, to satisfy the type `registry_path: Option<impl AsRef<Utf8Path>>`. At least, this is the best I could do.
| @@ -522,10 +522,13 @@ impl ChainEndpoint for PenumbraChain { | |||
| } | |||
| }; | |||
|
|
|||
| // No support for custom registry.json files in Hermes yet. | |||
| let registry_path: Option<camino::Utf8PathBuf> = None; | |||
There was a problem hiding this comment.
Is there a way to avoid pulling in the camino crate as a workspace dep?
There was a problem hiding this comment.
Is there a reason to? It's useful
There was a problem hiding this comment.
Ah, my intent wasn't clear. I should have asked: "Is explicitly declaring the type Option<camino::Utf8PathBuf> the minimal change necessary to satisfy?"
There was a problem hiding this comment.
registry_path is an Option<impl AsRef<Utf8Path>> and String implements AsRef<Utf8Path> so we can:
// No support for custom registry.json files in Hermes yet.
let registry_path: Option<String> = None;There was a problem hiding this comment.
Thanks, that's exactly what I was reaching for.
rust-toolchain.toml
Outdated
| [toolchain] | ||
| # Pin Rust 1.79 to avoid API breakage in `time` crate. | ||
| channel = "1.79" | ||
| components = [ "rustfmt", "rust-analyzer" ] |
There was a problem hiding this comment.
Rather than pin old rust, maybe we should just add boxing where recommended. I'm sure that's happened in the upstream Hermes already.
There was a problem hiding this comment.
They did remove their rust-toolchain.toml file: informalsystems#2706
I'm not sure what the time crate-related API breakage is, though
There was a problem hiding this comment.
Looks like they just set the msrv in the Cargo.toml files directly: https://github.com/informalsystems/hermes/blob/master/crates/relayer/Cargo.toml#L10
We have similar but on a different version: https://github.com/penumbra-zone/hermes/blob/main/crates/relayer/Cargo.toml#L5
There was a problem hiding this comment.
I'm not sure what the
timecrate-related API breakage is, though
If you check out the main branch on this repo, without a rust-toolchain.toml file, and run cargo check, you'll see:
error[E0282]: type annotations needed for `Box<_>`
--> /home/conor/.cargo/registry/src/index.crates.io-6f17d22bba15001f/time-0.3.34/src/format_description/parse/mod.rs:83:9
|
83 | let items = format_items
| ^^^^^
...
86 | Ok(items.into())
| ---- type must be known at this point
|
= note: this is an inference error on crate `time` caused by an API change in Rust 1.80.0; update `time` to version `>=0.3.35` by calling `cargo
update`
There was a problem hiding this comment.
Hm I can cargo check on this branch without the rust-toolchain.toml file. This might be because of the Rust version I have installed?
$ rustc -V
rustc 1.82.0 (f6e511eec 2024-10-15)
There was a problem hiding this comment.
I pushed a commit to remove the rust-toolchain.toml and update the msrv to 1.79.0 everywhere
Marking ready for review because the code we want is present and we want to move forward with testing. |
Bumping the Penumbra dependencies to
v0.80.x, specifically on the current main branch of the protocol repo. Ideally we'd tag av0.80.10, containing fixes from penumbra-zone/penumbra#4905, but we haven't done that just yet.Notably, the bump in Penumbra protocol deps will involve a schema change for the view database. I haven't tested locally yet, but I expect Hermes when upgraded to throw an error about the view schema mismatch. If and when that happens during interactive testing on the testnet, I'll make sure to document those errors so we can communicate the need to perform maintenance to operators.